-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Added multi-project feature #34386
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Added multi-project feature #34386
Conversation
Hi @lunny Could you please review and confirm the issue on db-tests? |
The test failure is related. This one has been added to my review list. |
Fixed unit test issues |
Add my test results video Gitea Multiple Projects Feature Test Steps |
This reverts commit 7e6750e.
Any questions? |
var p project_model.Project | ||
has, err := db.GetEngine(ctx).Table("project"). | ||
func (issue *Issue) LoadProjects(ctx context.Context) (err error) { | ||
if len(issue.Projects) == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It causes duplicate "loads".
If should check issue.Projects == nil
, and below it should set issue.Projects
to an empty slice but not nil if there is no project.
return 0 | ||
func (issue *Issue) projectIDs(ctx context.Context) []int64 { | ||
var ids []int64 | ||
if err := db.GetEngine(ctx).Table("project_issue").Where("issue_id=?", issue.ID).Select("project_id").Find(&ids); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not safe to ignore the err, in case the SQL would be wrong.
@@ -96,71 +90,88 @@ func LoadIssuesFromColumn(ctx context.Context, b *project_model.Column, opts *Is | |||
|
|||
// IssueAssignOrRemoveProject changes the project associated with an issue | |||
// If newProjectID is 0, the issue is removed from the project | |||
func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_model.User, newProjectID, newColumnID int64) error { | |||
func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_model.User, newProjectIDs []int64, newColumnID int64) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How could it be right?
Why newColumnID
could apply to all newProjectIDs
?
@@ -30,7 +30,7 @@ type IndexerData struct { | |||
LabelIDs []int64 `json:"label_ids"` | |||
NoLabel bool `json:"no_label"` // True if LabelIDs is empty | |||
MilestoneID int64 `json:"milestone_id"` | |||
ProjectID int64 `json:"project_id"` | |||
ProjectIDs []int64 `json:"project_id"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think the change is right for indexers, I am pretty sure it breaks existing indexers and the JSON field name doesn't match.
return a[:n] | ||
} | ||
|
||
func JoinSlice[T any](items []T, toString func(T) string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It duplicates with Int64sToStrings
and StringsToInt64s
?
} else { | ||
ctx.JSONRedirect(project_model.ProjectLinkForRepo(repo, project.ID)) | ||
if ctx.FormString("redirect_after_creation") == "project" && len(projectIDs) > 0 { | ||
for _, projectID := range projectIDs { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can't be right, why "redirect" to every project's link?
Reopening PR due to branch rename
Close #30381 #12974 #27984 #30163
Add my test results video Gitea Multiple Projects Feature Test Steps